Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

config: fix file/folder ownership #2745

Merged
merged 1 commit into from
Oct 23, 2024
Merged

config: fix file/folder ownership #2745

merged 1 commit into from
Oct 23, 2024

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Oct 17, 2024 •
edited
Loading

alternative and closes #2739

When buildx runs through sudo, file and folder created are owned by root user. This is fine if HOME is not kept in /etc/sudoers config which is the default behavior on unix systems and would therefore write to /root but this is not the case on macOS:

Defaults        env_keep += "HOME MAIL"

$ sudo env|sort
HOME=/Users/crazymax
LOGNAME=root
MAIL=/var/mail/root
PATH=...
SHELL=/bin/sh
SUDO_COMMAND=/usr/bin/env
SUDO_GID=20
SUDO_UID=502
SUDO_USER=crazymax
TERM=xterm-256color
USER=root

Checked as well when using macos GitHub Runners:

image

If we try to create a builder with sudo, it would write with root owner in user's home:

$ sudo docker buildx create --name foo
foo
$ tree -pugfi ~/.docker/buildx
[drwx------ crazymax staff   ]  /Users/crazymax/.docker/buildx
[drwx------ crazymax staff   ]  /Users/crazymax/.docker/buildx/activity
[-rw------- crazymax staff   ]  /Users/crazymax/.docker/buildx/activity/desktop-linux
[-rw------- root     staff   ]  /Users/crazymax/.docker/buildx/activity/foo
[-rw------- crazymax staff   ]  /Users/crazymax/.docker/buildx/current
[drwx------ crazymax staff   ]  /Users/crazymax/.docker/buildx/defaults
[drwx------ crazymax staff   ]  /Users/crazymax/.docker/buildx/instances
[-rw------- root     staff   ]  /Users/crazymax/.docker/buildx/instances/foo

If we then try to remove this builder without sudo we got:

$ docker buildx rm foo
failed to remove foo: open /Users/crazymax/.docker/buildx/instances/foo: permission denied
ERROR: failed to remove one or more builders

Same if try to list builders:

$ docker buildx ls
ERROR: open /Users/crazymax/.docker/buildx/instances/foo: permission denied

To fix this issue and avoid changing file permissions like #2739, we need to check if buildx is running through sudo and set right owner when writing file or folder in configuration directory.

@crazy-max crazy-max changed the title set right ownership on file/folder when invoking with sudo config: fix file/folder ownership Oct 18, 2024
@crazy-max crazy-max force-pushed the detect-sudo branch 5 times, most recently from dfd5ccf to fa17048 Compare October 18, 2024 11:31
@crazy-max crazy-max marked this pull request as ready for review October 18, 2024 11:40
@crazy-max crazy-max added this to the v0.18.0 milestone Oct 18, 2024
tonistiigi
tonistiigi reviewed Oct 21, 2024
View reviewed changes
util/confutil/config.go Outdated Show resolved Hide resolved
util/confutil/config_unix.go Outdated Show resolved Hide resolved
util/confutil/config.go Outdated Show resolved Hide resolved
util/confutil/config.go Outdated Show resolved Hide resolved
util/confutil/config.go Outdated Show resolved Hide resolved
crazy-max
crazy-max commented Oct 22, 2024
View reviewed changes
util/confutil/config.go Outdated Show resolved Hide resolved
@crazy-max crazy-max mentioned this pull request Oct 22, 2024
Merged
tonistiigi
tonistiigi reviewed Oct 22, 2024
View reviewed changes
util/confutil/config.go Outdated Show resolved Hide resolved
@crazy-max crazy-max force-pushed the detect-sudo branch 2 times, most recently from 6d17375 to 681d93d Compare October 22, 2024 21:19
tonistiigi
tonistiigi reviewed Oct 22, 2024
View reviewed changes
util/confutil/config.go Outdated Show resolved Hide resolved
util/confutil/config.go Outdated Show resolved Hide resolved
@tonistiigi tonistiigi merged commit 746eadd into docker:master Oct 23, 2024
107 checks passed
@crazy-max crazy-max deleted the detect-sudo branch October 23, 2024 18:09
@crazy-max crazy-max mentioned this pull request Nov 1, 2024
Closed
3 tasks
@thaJeztah thaJeztah mentioned this pull request Nov 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonistiigi tonistiigi tonistiigi approved these changes

Assignees
No one assigned
Labels
area/build area/builder area/cli area/controller area/dependencies area/tests
Projects
None yet
Milestone
v0.18.0
2 participants
@crazy-max @tonistiigi